Cookie-based auth #1521

dokterbob · 2024-11-14T14:46:48Z

Implementation of #1520

dokterbob · 2024-11-19T14:43:17Z

There's currently E2E test failures in:

chat_profiles
data_layer
header_auth
password_auth

This is kind of to be expected, given the scale of this refactor.

In addition, we need to make sure that files are served from a place which does not have API access, e.g. the files should really be untrusted. Otherwise, an LLM or whoever uploads files could call the Chainlit API on the user's behalf by crafting malicious HTML with JS.

To get there, we need to:

Serve the API from a single root/prefix, e.g. /api/.
Serve files from another, e.g. /files/.
Set auth cookie with path=/api/, allowing acces to the API only. Including file uploads.
Set another auth cookie with path=/files/, allowing only GET (and perhaps HEAD) to files.
For legacy support, add a permanent redirect from the current file API location to the new location (and give a deprecation warning when redirecting).

This would be a good moment to 'go all in' in terms of file security. We could also postpone this to a later PR and/or explicitly document that files in their current implementation should not come from untrusted sources (e.g. AI-generated or from 3rd parties).

dokterbob · 2024-11-28T22:58:59Z

Just 2 more tests missing:

────────────────────────────────────────────────────────────────────────────────────────────────────
                                                                                                    
  Running:  header_auth/spec.cy.ts                                                        (18 of 37)


  Header auth
    1) should fail to auth without custom header
    ✓ should be able to auth with custom header (6430ms)


  1 passing (16s)
  1 failing

  1) Header auth
       should fail to auth without custom header:
     CypressError: The application redirected to `http://127.0.0.1:8000/login` more than 20 times. Please check if it's an intended behavior.

If so, increase `redirectionLimit` value in configuration.
      at onWindowLoad (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:137531:80)
      at $Cy.onInternalWindowLoad (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:137563:20)
      at $Cy.listener (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:86503:17)
      at ../driver/node_modules/eventemitter2/lib/eventemitter2.js.EventEmitter.emit (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:86583:19)
      at parent.<computed> [as emit] (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:155168:32)
      at $Cypress.action (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:147734:14)
      at HTMLIFrameElement.<anonymous> (http://127.0.0.1:8000/__cypress/runner/cypress_runner.js:151065:24)
  From Your Spec Code:
      at Context.eval (webpack:///./cypress/e2e/header_auth/spec.cy.ts:9:0)




  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        2                                                                                │
  │ Passing:      1                                                                                │
  │ Failing:      1                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  1                                                                                │
  │ Video:        false                                                                            │
  │ Duration:     15 seconds                                                                       │
  │ Spec Ran:     header_auth/spec.cy.ts                                                           │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


  (Screenshots)

  -  /home/runner/work/chainlit/chainlit/cypress/screenshots/header_auth/spec.cy.ts/H     (1280x720)
     eader auth -- should fail to auth without custom header (failed).png                           

────────────────────────────────────────────────────────────────────────────────────────────────────
                                                                                                    
  Running:  data_layer/spec.cy.ts                                                         (12 of 37)


  Data Layer
    Data Features with persistence
      1) should login, submit feedback, wait for user input to create steps, browse thread history, delete a thread and then resume a thread
      2) should continue the thread after backend restarts and work with new thread as usual


  0 passing (41s)
  2 failing

  1) Data Layer
       Data Features with persistence
         should login, submit feedback, wait for user input to create steps, browse thread history, delete a thread and then resume a thread:
     AssertionError: Timed out retrying after 15000ms: expected '/' to match /^\/thread\//
      at Context.eval (webpack:///./cypress/e2e/data_layer/spec.cy.ts:16:0)

  2) Data Layer
       Data Features with persistence
         should continue the thread after backend restarts and work with new thread as usual:
     AssertionError: Timed out retrying after 15000ms: expected '/' to match /^\/thread\//
      at Context.eval (webpack:///./cypress/e2e/data_layer/spec.cy.ts:16:0)




  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        2                                                                                │
  │ Passing:      0                                                                                │
  │ Failing:      2                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  2                                                                                │
  │ Video:        false                                                                            │
  │ Duration:     40 seconds                                                                       │
  │ Spec Ran:     data_layer/spec.cy.ts                                                            │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘


  (Screenshots)

  -  /home/runner/work/chainlit/chainlit/cypress/screenshots/data_layer/spec.cy.ts/Da     (1280x720)
     ta Layer -- Data Features with persistence -- should login, submit feedback, wai               
     t for user input to create steps, browse thread history, delete a thread and the               
     n resume a thread (failed).png                                                                 
  -  /home/runner/work/chainlit/chainlit/cypress/screenshots/data_layer/spec.cy.ts/Da     (1280x720)
     ta Layer -- Data Features with persistence -- should continue the thread after b               
     ackend restarts and work with new thread as usual (failed).png                                 


────────────────────────────────────────────────────────────────────────────────────────────────────```

* Organise imports and formatting. * Log uncaught exceptions in data layer. * Typing cleanup and explicit assertions. * Move auth to module. * Factor out JWT functionality.

* Factor out redundant maybe user creating. * Factor out common user param. * Add a get_user() route to allow access to user data. * Import sorting. * Make using cookies the default. * Refuse to serve files unless we are using cookie auth.

Prevents redundant redirect.

* Header and password auth support. * Consistent typing for useAuth(). * Comment to document useAPI() hook. * Factor out auth types, config, state, settings, session and token management. * Determine auth state based on whether user is set. * Make auth a core component module. * Check `/login/` response for expected output. * Request `/user` when cookieAuth's available. * Use `user` instead of `access_token` to determine whether we're logged in.

* Don't assume errors are always JSON with .detail * Add status to ClientError. * Factor out request error handling. * Consistent handling of errors: either call on401() or throw, never both. * Disable/override on4041 and onError hooks for useApi(). Requests not initiated by the user should not cause user-facing notifications. Plus, 401 on `/user` is now expected behaviour.

* Separate assertions and state preparation. * Check for call on /user for cookie auth. * Test for presence of cookie in login reply.

dokterbob · 2024-12-05T16:33:04Z

Anyone a Windows box lying around? 🤯

dokterbob · 2024-12-05T16:56:48Z

@hmrc87 @celeriev Would love initial feedback. Might have been stuff I missed, kind of a large chunk of work. ;)

dokterbob · 2024-12-09T22:53:53Z

Copilot auth

Option A

Simplified OAuth, assuming that the owner of client_secret can be trusted. JWT token contains client_id and is signed by client_secret. Server needs to be configured to allow client_id, which is used to authenticate client_secret. JWT exchange token is very short lived (minutes).

Here's a roll-up of the authentication system:

Secure Embedded App Authentication

This system enables secure authentication for embedded applications using Ed25519 public-key cryptography and HTTPOnly cookies, balancing security with a simple developer experience.

Overview

Each integration partner gets:
- A private key (client_secret)
- A corresponding public key (serves as client_id)
Authentication Flow:
- Host server generates a JWT containing:
  - User identity/permissions
  - Their public key
  - Signed with their private key
- Embedded app initializes with this JWT
- API validates JWT using public key from the token
- Upon validation, sets secure HTTPOnly cookie
- Further requests use cookie auth

Security Properties

Asymmetric crypto (Ed25519) ensures hosts can only sign for themselves
Self-contained tokens (pubkey included) with minimal server config
Server whitelists allowed client_ids for access control
HTTPOnly cookies prevent XSS after initial auth
256-bit Ed25519 keys provide strong security with small size

Key Benefits

Zero server config for new integrations (beyond client_id whitelist)
Clean developer experience
Single auth exchange at session start
Clear audit trail (client_id identifies signer)
Strong security model with minimal complexity

Limitations

Initial JWT briefly exposed to JavaScript
Requires hosts to securely store private key
Need to maintain client_id whitelist

Option B

Server calling out to copilot uses OAuth to authenticate with chainlit, chainlit generates a short-lived refresh token, server shares it with the client, client exchanges for a cookie.

This is the most standardised but also adds a considerable amount of complexity.

The key points:

Host server uses client credentials + custom claims for user context
Short-lived initial token only used once to establish cookie
After exchange, everything works with HTTPOnly cookies
User never sees auth process
Maintains security while being invisible to end user

dokterbob · 2024-12-09T22:55:55Z

I hugely appreciate community feedback and/or a security review of the aforementioned options for cookie auth. @hmrc87 @celeriev @willydouhard @robkrause

willydouhard · 2024-12-13T08:04:52Z

backend/chainlit/auth/cookie.py

+
+        # If no cookie, try the Authorization header as fallback
+        if not token:
+            # TODO: Only bother to check if cookie auth is explicitly disabled.


are we leaving this todo?

willydouhard · 2024-12-13T08:14:18Z

backend/chainlit/server.py

-    user = await config.code.password_auth_callback(
-        form_data.username, form_data.password
+        return RedirectResponse(
+            # FIXME: redirect to the right frontend base url to improve the dev environment


should we fix this before RC?

dokterbob force-pushed the dokterbob/cookie-auth branch 2 times, most recently from c49b0d8 to 38f7ea9 Compare November 19, 2024 14:02

dokterbob force-pushed the dokterbob/cookie-auth branch 3 times, most recently from 83a4474 to 441d198 Compare November 27, 2024 22:18

This was referenced Dec 3, 2024

Add option for custom auth #1280

Open

Feature - Show images in a LightBox when clicked #1473

Open

dokterbob force-pushed the dokterbob/cookie-auth branch from 272f985 to 6bc5694 Compare December 5, 2024 15:07

dokterbob added 6 commits December 5, 2024 15:25

Add single lint command for convenience.

2fc0d9d

Allow and document passing arbitrary options to Cypress.

12cd519

Cleanup of auth module.

dc34f2f

* Organise imports and formatting. * Log uncaught exceptions in data layer. * Typing cleanup and explicit assertions. * Move auth to module. * Factor out JWT functionality.

Remove deprecated ruff rule.

91f078e

Bump ruff and use consistent version in CI.

452c456

Backend implementation of cookie auth.

6bf4119

* Factor out redundant maybe user creating. * Factor out common user param. * Add a get_user() route to allow access to user data. * Import sorting. * Make using cookies the default. * Refuse to serve files unless we are using cookie auth.

dokterbob force-pushed the dokterbob/cookie-auth branch from 6bc5694 to 4d81a99 Compare December 5, 2024 15:44

dokterbob added 6 commits December 5, 2024 16:06

Only redirect to /login once auth config is available.

bc966d9

Prevents redundant redirect.

Expose cookie auth config to frontend.

818197e

Move onErrorRetry logic to useAPI(), prevents infinite recursion.

b5c5b60

Cleanup, improvement and cookie auth tests in password_* tests.

84e88ad

* Separate assertions and state preparation. * Check for call on /user for cookie auth. * Test for presence of cookie in login reply.

dokterbob force-pushed the dokterbob/cookie-auth branch from 4d81a99 to 84e88ad Compare December 5, 2024 16:10

Re-enable user validation for get_file(). Closes #1101.

4f8bc3f

dokterbob requested a review from willydouhard December 11, 2024 12:31

dokterbob added the review-me Ready for review! label Dec 11, 2024

dokterbob mentioned this pull request Dec 12, 2024

Maintaining session state between different html pages for Copilot #988

Open

willydouhard reviewed Dec 13, 2024

View reviewed changes

willydouhard added 6 commits December 13, 2024 13:58

fix: conflicts with sticky session PR

6655a5b

fix: ruff formatting

10339cb

fix: test

a71f2e0

fix: ruff

6d82140

fix: test

f19e4dd

fix: test

f9d2538

willydouhard marked this pull request as ready for review December 13, 2024 19:54

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. auth Pertaining to authentication. labels Dec 13, 2024

willydouhard merged commit cefc8d9 into main Dec 13, 2024
9 checks passed

willydouhard deleted the dokterbob/cookie-auth branch December 13, 2024 19:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cookie-based auth #1521

Cookie-based auth #1521

dokterbob commented Nov 14, 2024 •

edited

Loading

dokterbob commented Nov 19, 2024 •

edited

Loading

dokterbob commented Nov 28, 2024

dokterbob commented Dec 5, 2024

dokterbob commented Dec 5, 2024

dokterbob commented Dec 9, 2024

dokterbob commented Dec 9, 2024

willydouhard Dec 13, 2024

willydouhard Dec 13, 2024

Cookie-based auth #1521

Cookie-based auth #1521

Conversation

dokterbob commented Nov 14, 2024 • edited Loading

dokterbob commented Nov 19, 2024 • edited Loading

dokterbob commented Nov 28, 2024

dokterbob commented Dec 5, 2024

dokterbob commented Dec 5, 2024

dokterbob commented Dec 9, 2024

Copilot auth

Option A

Secure Embedded App Authentication

Overview

Security Properties

Key Benefits

Limitations

Option B

dokterbob commented Dec 9, 2024

willydouhard Dec 13, 2024

Choose a reason for hiding this comment

willydouhard Dec 13, 2024

Choose a reason for hiding this comment

dokterbob commented Nov 14, 2024 •

edited

Loading

dokterbob commented Nov 19, 2024 •

edited

Loading